VS-402. Add VAT Validation check that aa_change and exon_number are consistentally set. #7850
Conversation
Codecov Report
@@ Coverage Diff @@
## ah_var_store #7850 +/- ##
================================================
Coverage ? 86.296%
Complexity ? 35197
================================================
Files ? 2170
Lines ? 164876
Branches ? 17783
================================================
Hits ? 142282
Misses ? 16270
Partials ? 6324 |
gbggrant
force-pushed
the
gg_VS-402_ValidateForAAChange
branch
from
fefa78d
to
9e65874
Compare
scripts/variantstore/variant_annotations_table/GvsValidateVAT.wdl
Outdated
Show resolved
Hide resolved
| if [[ $NUMVARS = "0" ]]; then | ||
| echo "PASS: The VAT table ~{fq_vat_table} has consistency across all aa_change and exon_number values in it." > validation_results.txt | ||
| else | ||
| echo "FAIL: The VAT table ~{fq_vat_table} has $NUMVARS variants for which aa_change and exon_number are inconsistent." > validation_results.txt |
There was a problem hiding this comment.
should there be an exit 1 here?
There was a problem hiding this comment.
No, the way the code is written all of the validation tasks just put their output into a results file. The workflow itself does not fail if it fails validation. It might make sense to do so, but I'm following the pattern here that @RoriCremer is using.
There was a problem hiding this comment.
Definitely curious to hear what @RoriCremer thinks, personally I like validations that make some obvious noise when things don't check out. 🙂
There was a problem hiding this comment.
Agreed---I like obvious failures, but I'd prefer that all the tests run
| if [[ $NUMVARS = "8" ]]; then | ||
| echo "PASS: The VAT table ~{fq_vat_table} has been spot checked for aa_change and exon_number consistency." > validation_results.txt | ||
| else | ||
| echo "FAIL: The VAT table ~{fq_vat_table} has failed the spot check for aa_change and exon_number consistency." > validation_results.txt |
There was a problem hiding this comment.
Same situation as described above.
There was a problem hiding this comment.
I'm a bit torn here---I think this should always pass for AoU, but if we have other users, the VAT might not have all of these variants
(I think that's fine for now)
gbggrant
force-pushed
the
gg_VS-402_ValidateForAAChange
branch
from
83ac402
to
45eb768
Compare
| } | ||
| # Check that cases where (aa_change non-null AND exon_number null) OR (aa_change null AND exon_number non-null) | ||
| # are all accounted for / understood. | ||
|
|
There was a problem hiding this comment.
nit: Do we want to put anything here about "understood" and all these like edge cases?
gbggrant
force-pushed
the
gg_VS-402_ValidateForAAChange
branch
from
d94afbe
to
bc1318f
Compare
…tly set. Added the spot-check test. Modified the way that the final report is generated, creating a task that generates the report from all of the validation results.
gbggrant
force-pushed
the
gg_VS-402_ValidateForAAChange
branch
from
0ad3c70
to
5b1eb60
Compare
|
@mcovarr @RoriCremer I have modified this now to fail outright if one of the validations fail. It now calls a GenerateFinalReport task as its last task and that will summarize the results in a way that (hopefully) makes it easier to understand the validation failure. |
Add VAT Validation check that aa_change and exon_number are consistentally set.